Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LEAF-4614 - double slash #2658

Merged
merged 5 commits into from
Jan 22, 2025

Conversation

shaneodd
Copy link
Contributor

@shaneodd shaneodd commented Jan 17, 2025

Double Slash

https://nginx.org/en/docs/http/ngx_http_core_module.html#merge_slashes
Internally is handles the double slashes, but does not always match properly when working with query strings. I am not finding the original document that talked about the query strings not always matching. However is apparent with URLs like the one below.

https://host.docker.internal/Test_Request_Portal/admin//?a=mod_templates&file=view_homepage.tpl

Then you have items like below that work as expected with double slashes

https://host.docker.internal/Test_Request_Portal//report.php?a=LEAF_Data_Visualizer&query=N4IgLgpgTgtgziAXAbVASwCZJAYwIaQDmA9lAJ4CSAIiADQjEAO0Bp2AvHSDATgBbYAZqRgB9AKwQ8ABgDsXQgQjYAggDkaAX1rosiEBggAbCJCz0mLMG32d6PMPyTT6iyKo0htu7BiUBlAFcAIxg0MDMuSyhWKGwAPjtuXgF9AEYAJgB6NIAOLIzpDIAWBSUPLQBdegArYjQAOwQUEDgwAkCEatbSMCRgbRBCUyoCPCRkAGZaYtpxWgA2WllaXNo0lwBOSs0gA%3D

https://superuser.com/questions/565250/how-to-remove-double-slash-in-urls-served-by-nginx

if ($request_uri ~ "//") {
     return 301 $uri$is_args$args;
}

The above code with merge_slashes on will redirect to the proper URL. This will redirect with query strings. It redirects properly since the URL internally has all of the slashes combined internally.

Manual Tests

https://host.docker.internal/Test_Request_Portal/admin//?a=mod_templates&file=view_homepage.tpl will redirect to https://host.docker.internal/Test_Request_Portal/admin/a=mod_templates&file=view_homepage.tpl

https://host.docker.internal/Test_Request_Portal/report.php?a=LEAF_import_data will redirect to https://host.docker.internal/Test_Request_Portal/report.phpa=LEAF_import_data

Test

Now that we have VAPO merged on this corrected this test

API

Updating DB Schema: Request Portal... OK
Updating DB Schema: Local Nexus (Orgchart)... OK
Updating DB Schema: National Nexus (Orgchart)... OK
PASS
ok  	LEAF/API-tester	20.338s

E2E

image

Pelentan
Pelentan previously approved these changes Jan 21, 2025
jampaul3
jampaul3 previously approved these changes Jan 22, 2025
@shaneodd shaneodd dismissed stale reviews from jampaul3 and Pelentan via 7defdbe January 22, 2025 14:17
@Pelentan Pelentan changed the base branch from master to rc/2025-01-22/Sprint-88-c1 January 22, 2025 16:05
@Pelentan Pelentan merged commit 6157fcf into rc/2025-01-22/Sprint-88-c1 Jan 22, 2025
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants